Antipattern: Making Bricks Without Straw

Developers commonly practice the See No Evil antipattern in two forms: first, ignoring the return values of a database API, and second, reading fragments of SQL code interspersed with application code. In both cases, developers fail to use information that is readily available to them.

Diagnoses without diagnostics#

Let’s take a look at the code below:

<?php
＄pdo = new PDO("mysql:dbname=test;host=db.example.com",
"dbuser", "dbpassword");
＄sql = "SELECT bug_id, summary, date_reported FROM Bugs
WHERE assigned_to = ? AND status = ?";  //1
＄stmt = ＄dbh->prepare(＄sql);            //2
＄stmt->execute(array(1, "OPEN"));       //3
＄bug = ＄stmt->fetch();                  //4
?>

This code is concise, but there are several places in this code where status values returned from functions could indicate a problem. However, we’ll never know about the problem if we ignore the return values.

One of the most common errors in a database API occurs when we try to create a database connection, for example, at point 1. Any of the following could happen. We could accidentally mistype the database name or server hostname, or we could get the username or password wrong, or the database server could be unreachable. An error in instantiating a PDO connection may throw an exception, which could terminate the example script shown previously.

The call to prepare() at point 2 could return false if a simple syntax error is caused by a typo, an imbalanced parenthesis, or a misspelled column name. If this happens, the attempt to call execute() as a method of ＄stmt at point 3 would be a fatal error because the value false isn’t an object.

PHP fatal error: Call to a member function execute() on a non-object

The call to execute() could also fail, for example, because the statement violates a constraint or exceeds access privileges. The method also returns false on error.

The call to fetch() at point 4 would return false if any other error occurs, such as if the connection to the RDBMS fails.

Programmers with attitudes like Mr. Davis aren’t uncommon. They may feel that checking return values and exceptions add nothing to their code because those cases aren’t supposed to happen anyway. Moreover, the extra code is repetitive and makes an application ugly and hard to read in their eyes. It definitely adds no coolness.

But users don’t see the code; they only see the output. When a fatal error goes unhandled, the user may see only a blank white screen, as in the figure given below, or an incomprehensible exception message. When this happens, it’s little consolation that the application code is tidy and concise.

Lines between the reading#

Another common bad habit that fits the See No Evil antipattern is to debug by staring at application code that builds an SQL query as a string. This is difficult because it’s hard to visualize the resulting SQL string after building it with application logic, string concatenation, and extra content from application variables.

Trying to debug in this way is like solving a jigsaw puzzle without looking at the photo on the box.

For a simple example, let’s look at a type of question we frequently see from developers. The following code builds a query conditionally by concatenating a WHERE clause if the script needs to search for a specific bug instead of a collection of bugs.

<?php 
＄sql = "SELECT * FROM Bugs"; 
if (＄bug_id) { 
   ＄sql .= "WHERE bug_id = " . intval(＄bug_id); 
}
＄stmt = ＄pdo->prepare(＄sql);
?>

Why would the query in this example give an error? The answer is clearer if we look at the full ＄sql string resulting from the concatenation:

There’s no whitespace between Bugs and WHERE, which gives the query invalid syntax as though it were reading a table called BugsWHERE, followed by an SQL expression in an invalid context. The code concatenates the strings with no space between them.

Developers waste an unbelievable amount of time and energy trying to debug problems like this by looking at the code that builds the SQL instead of looking at the SQL itself.